Skip to content

test: fixed the execution for the threadsfafe function test.#807

Closed
NickNaso wants to merge 1 commit intonodejs:masterfrom
NickNaso:issue-806
Closed

test: fixed the execution for the threadsfafe function test.#807
NickNaso wants to merge 1 commit intonodejs:masterfrom
NickNaso:issue-806

Conversation

@NickNaso
Copy link
Member

@NickNaso NickNaso commented Sep 3, 2020

Fixes: #806

@NickNaso NickNaso mentioned this pull request Sep 4, 2020
Closed
6 tasks
@mhdawson
Copy link
Member

mhdawson commented Sep 4, 2020

Can you add a bit more detail on what you saw in terms of it not running?

When I run the tests before apply the patch I see:

Running test 'threadsafe_function/threadsafe_function_ctx'
Running test 'threadsafe_function/threadsafe_function_existing_tsfn'
Running test 'threadsafe_function/threadsafe_function_ptr'
Running test 'threadsafe_function/threadsafe_function_sum'
Running test 'threadsafe_function/threadsafe_function_unref'
Running test 'threadsafe_function/threadsafe_function'

@NickNaso
Copy link
Member Author

NickNaso commented Sep 4, 2020

Hi @mhdawson,
what you see printed on the console is in https://github.com/nodejs/node-addon-api/blob/master/test/index.js#L106 but this does not mean that the corresponding test is executed. In the test for threadsafe function https://github.com/nodejs/node-addon-api/blob/master/test/threadsafe_function/threadsafe_function.js#L7 we are exporting only the function instead we should export a a function invocation. Trying to insert a console.log('...') like reported below:

module.exports = async function() {
  console.log('Execution of test for threadsafe function');
  await test(require(`../build/${buildType}/binding.node`));
  await test(require(`../build/${buildType}/binding_noexcept.node`));
};

you could see that the test will not be executed. This happened in the PR #787.

@mhdawson
Copy link
Member

mhdawson commented Sep 4, 2020

@NickNaso thanks for the clarification.

mhdawson
mhdawson approved these changes Sep 4, 2020
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

mhdawson pushed a commit that referenced this pull request Sep 4, 2020
@NickNaso @mhdawson
test: fix the threasfafe function test
a13b36c 
test: fixed the execution for the threasfafe function test

PR-URL: #807
Fixes: #806
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
-sh-4.2$
@mhdawson
Copy link
Member

mhdawson commented Sep 4, 2020

Landed as a13b36c

@mhdawson mhdawson closed this Sep 4, 2020
Superlokkus pushed a commit to Superlokkus/node-addon-api that referenced this pull request Nov 20, 2020
@NickNaso @Superlokkus
test: fix the threasfafe function test
3bc5db4 
test: fixed the execution for the threasfafe function test

PR-URL: nodejs#807
Fixes: nodejs#806
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
-sh-4.2$
kevindavies8 added a commit to kevindavies8/node-addon-api-Develop that referenced this pull request Aug 24, 2022
@kevindavies8
test: fix the threasfafe function test
34b88ec 
test: fixed the execution for the threasfafe function test

PR-URL: nodejs/node-addon-api#807
Fixes: nodejs/node-addon-api#806
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
-sh-4.2$
Marlyfleitas added a commit to Marlyfleitas/node-api-addon-Development that referenced this pull request Aug 26, 2022
@Marlyfleitas
test: fix the threasfafe function test
59f9af1 
test: fixed the execution for the threasfafe function test

PR-URL: nodejs/node-addon-api#807
Fixes: nodejs/node-addon-api#806
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
-sh-4.2$
wroy7860 added a commit to wroy7860/addon-api-benchmark-node that referenced this pull request Sep 19, 2022
@wroy7860
test: fix the threasfafe function test
fd97288 
test: fixed the execution for the threasfafe function test

PR-URL: nodejs/node-addon-api#807
Fixes: nodejs/node-addon-api#806
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
-sh-4.2$
johnfrench3 pushed a commit to johnfrench3/node-addon-api-git that referenced this pull request Aug 11, 2023
test: fix the threasfafe function test
d44b882 
test: fixed the execution for the threasfafe function test

PR-URL: nodejs/node-addon-api#807
Fixes: nodejs/node-addon-api#806
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
-sh-4.2$
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mhdawson mhdawson mhdawson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

test: threadsafe function test is not executed after last changes

2 participants

@NickNaso @mhdawson